-
Notifications
You must be signed in to change notification settings - Fork 801
Python: [Breaking] Python: Respond with AgentRunResponse with serialized structured output
#2285
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
AgentRunResponse with serialized structured outputAgentRunResponse with serialized structured output
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a breaking change to make DurableAIAgent.run() return AgentRunResponse instead of raw dictionaries, aligning with the agent-framework protocols and bringing parity with the .NET experience. The key changes include:
- Modified
DurableAIAgent.run()to use generator protocol (yield from) and return typedAgentRunResponse - Removed the
AgentResponsewrapper class in favor of usingAgentRunResponsedirectly with metadata inadditional_properties - Deserialization of structured output (
response.value) now happens in the orchestration layer via_ensure_response_format() - Updated all samples and tests to use
yield fromsyntax and access structured responses via.valueproperty
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| python/packages/azurefunctions/agent_framework_azurefunctions/_orchestration.py | Added generator-based run() method returning AgentRunResponse, with helper methods _load_agent_response() and _ensure_response_format() for response handling and structured output parsing |
| python/packages/azurefunctions/agent_framework_azurefunctions/_entities.py | Modified run_agent() to return AgentRunResponse directly, removed JSON parsing logic (now handled at orchestration layer), updated metadata handling via additional_properties |
| python/packages/azurefunctions/agent_framework_azurefunctions/_models.py | Removed AgentResponse dataclass as it's replaced by AgentRunResponse |
| python/samples/getting_started/azure_functions/04_single_agent_orchestration_chaining/function_app.py | Updated to use yield from and access response via .text property |
| python/samples/getting_started/azure_functions/05_multi_agent_orchestration_concurrency/function_app.py | Updated to manually advance generators for concurrent execution, accessing results via .text property |
| python/samples/getting_started/azure_functions/06_multi_agent_orchestration_conditionals/function_app.py | Updated to use yield from and access structured responses via .value property, removed _coerce_structured() helper |
| python/samples/getting_started/azure_functions/07_single_agent_orchestration_hitl/function_app.py | Updated to use yield from and access structured responses via .value property with validation, removed _coerce_generated_content() helper |
| python/packages/azurefunctions/tests/test_orchestration.py | Added tests for _load_agent_response() helper method covering instance, serialized, and None cases |
| python/packages/azurefunctions/tests/test_models.py | Removed all tests for the deleted AgentResponse class |
| python/packages/azurefunctions/tests/test_entities.py | Updated assertions to check AgentRunResponse type and access metadata via additional_properties |
| python/packages/azurefunctions/tests/test_app.py | Updated assertions to check AgentRunResponse type and access metadata via additional_properties |
...n/samples/getting_started/azure_functions/07_single_agent_orchestration_hitl/function_app.py
Outdated
Show resolved
Hide resolved
...n/samples/getting_started/azure_functions/07_single_agent_orchestration_hitl/function_app.py
Outdated
Show resolved
Hide resolved
...les/getting_started/azure_functions/05_multi_agent_orchestration_concurrency/function_app.py
Outdated
Show resolved
Hide resolved
...les/getting_started/azure_functions/05_multi_agent_orchestration_concurrency/function_app.py
Outdated
Show resolved
Hide resolved
...les/getting_started/azure_functions/05_multi_agent_orchestration_concurrency/function_app.py
Show resolved
Hide resolved
python/packages/azurefunctions/agent_framework_azurefunctions/_orchestration.py
Show resolved
Hide resolved
python/packages/azurefunctions/agent_framework_azurefunctions/_orchestration.py
Outdated
Show resolved
Hide resolved
...les/getting_started/azure_functions/05_multi_agent_orchestration_concurrency/function_app.py
Show resolved
Hide resolved
...n/samples/getting_started/azure_functions/07_single_agent_orchestration_hitl/function_app.py
Outdated
Show resolved
Hide resolved
python/packages/azurefunctions/agent_framework_azurefunctions/_entities.py
Outdated
Show resolved
Hide resolved
python/packages/azurefunctions/agent_framework_azurefunctions/_entities.py
Outdated
Show resolved
Hide resolved
python/packages/azurefunctions/agent_framework_azurefunctions/_entities.py
Outdated
Show resolved
Hide resolved
python/packages/azurefunctions/agent_framework_azurefunctions/_entities.py
Outdated
Show resolved
Hide resolved
python/packages/azurefunctions/agent_framework_azurefunctions/_entities.py
Outdated
Show resolved
Hide resolved
python/packages/azurefunctions/agent_framework_azurefunctions/_entities.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
python/packages/azurefunctions/agent_framework_azurefunctions/_orchestration.py
Outdated
Show resolved
Hide resolved
python/packages/azurefunctions/agent_framework_azurefunctions/_orchestration.py
Show resolved
Hide resolved
python/packages/azurefunctions/agent_framework_azurefunctions/_entities.py
Outdated
Show resolved
Hide resolved
python/packages/azurefunctions/agent_framework_azurefunctions/_orchestration.py
Outdated
Show resolved
Hide resolved
python/packages/azurefunctions/agent_framework_azurefunctions/_orchestration.py
Show resolved
Hide resolved
python/packages/azurefunctions/agent_framework_azurefunctions/_orchestration.py
Show resolved
Hide resolved
cgillum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to the feedback below, can we open an issue to track updating the snippets lab (the one we used for Ignite) to account for these breaking changes?
| role=Role.ASSISTANT, contents=[ErrorContent(message=str(exc), error_code=type(exc).__name__)] | ||
| ) | ||
| error_message = ChatMessage( | ||
| role=Role.ASSISTANT, contents=[ErrorContent(message=str(exc), error_code=type(exc).__name__)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like you're creating a custom error response from the entity when something unexpected happens, but I don't believe we're doing this in .NET. I'm also not sure if this is the right thing for us to do since it hides the fact that an error occurred from Functions telemetry. Regardless of what design we think is best, we should do the same thing in both languages. Can you open an issue to review the implementation and track consistent error handling for agent invocations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, the experiences should be consistent.
Created an issue to track this separately - #2434
| chemist_result = None | ||
|
|
||
| try: | ||
| physicist_gen.send(task_results[0]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've never seen this direct access of generators and send in durable Python apps before, and I don't think it's intended that we make this a normal part of the programming model. Why is this needed, and can it be replaced with something more conventional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a comment at the top of this file, but this is basically only for the advance cases where the customers want true parallelism with invoking multi-agents with generators.
"""Fan out to two domain-specific agents and aggregate their responses.
Note: The generator protocol lets us extract the Durable Task objects so we
can pass them to task_all for true parallelism. If you only use `yield
from` on each run call, the agents execute sequentially.
For sequential execution, you can simply use:
physicist_result = yield from physicist.run(messages=prompt, thread=physicist_thread)
chemist_result = yield from chemist.run(messages=prompt, thread=chemist_thread)
"""The conventional method is defined throughout the rest of the samples with using the yield from model. The only catch here is that that convention is more synchronous -
email_result_raw = yield from email_agent.run(
messages=email_prompt,
thread=email_thread,
response_format=EmailResponse,
)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand what the sample is doing, but I don't understand why we can't use normal generator syntax. This is not something durable customers need to do in Python today: https://learn.microsoft.com/en-us/azure/azure-functions/durable/durable-functions-overview?tabs=in-process%2Cnodejs-v3%2Cv1-model&pivots=python#fan-in-out
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The difference is in the response type of the types. The context.task_all expects a list[TaskBase] but the agent.run responds with a Generator[.., .., AgentRunResponse] due to which it can't resolve properly. The main reason why this is happening is because we do some post-processing after we get the result back from the entity after we signal it using context.signal_entity() and return an object of AgentRunResponse. Earlier, we were just returning the same task that we got when executing teh signal_entity command without any post-processing.
result = yield self.context.call_entity(entity_id, "run_agent", run_request.to_dict())
logger.debug(
"[DurableAIAgent] Entity call completed for correlation_id %s",
correlation_id,
)
response = self._load_agent_response(result)
if response_format is not None:
self._ensure_response_format(response_format, correlation_id, response)
return response
Opened an issue #2435 |
Motivation and Context
DurableAIAgent.runfunction to respond withAgentRunResponserather thanAnyto be more in line with agent-framework's protocols.response.valueas part of the agent run, making it easier for the customer to author their codes.AgentResponse).Description
The changes bring parity with Dotnet experience and also with agent-framework in general.
This introduces a breaking change where now we need to use
yield fromwhen calling theDurableAIAgent(vs justyieldbefore) -Additionally, the responses are now of type
AgentRunResponseand the structured response is stored inAgentRunResponse.value(as opposed toresponse["structured_response"]before).Contribution Checklist